Reduce queries from unfillable updater #3107
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
alfetopito
reviewed
Sep 5, 2023
apps/cowswap-frontend/src/legacy/state/orders/updaters/UnfillableOrdersUpdater.ts
Outdated
Show resolved
Hide resolved
apps/cowswap-frontend/src/legacy/state/orders/updaters/UnfillableOrdersUpdater.ts
Outdated
Show resolved
Hide resolved
|
@anxolin build still failing because of the new error type
|
alfetopito
reviewed
Sep 7, 2023
apps/cowswap-frontend/src/legacy/state/orders/updaters/UnfillableOrdersUpdater.ts
Outdated
Show resolved
Hide resolved
apps/cowswap-frontend/src/legacy/state/orders/updaters/UnfillableOrdersUpdater.ts
Outdated
Show resolved
Hide resolved
I still need to rebase the other branch |
anxolin
force-pushed
the
reduce-queries-from-unfillable-updater
branch
from
1610fee
to
2aee35c
Compare
anxolin
commented
Sep 7, 2023
| const enoughBalance = hasEnoughBalanceAndAllowance({ | ||
| account, | ||
| amount: currencyAmount, | ||
| balances: balancesRef.current, |
There was a problem hiding this comment.
Now balances uses a ref to not re-render the function that often, but always use the latest version of the balances
anxolin
commented
Sep 7, 2023
| @@ -11,7 +11,7 @@ export const ContractDeploymentBlocks: Partial<Record<ChainId, number>> = { | |||
| export const MARKET_OPERATOR_API_POLL_INTERVAL = ms`2s` | |||
| // We can have lots of limit orders and it creates a high load, so we poll them no so ofter as market orders | |||
| export const LIMIT_OPERATOR_API_POLL_INTERVAL = ms`15s` | |||
| export const PENDING_ORDERS_PRICE_CHECK_POLL_INTERVAL = ms`15s` | |||
| export const PENDING_ORDERS_PRICE_CHECK_POLL_INTERVAL = ms`30s` | |||
There was a problem hiding this comment.
I don't think we need to poll this often
anxolin
commented
Sep 7, 2023
| updatePending() | ||
| const interval = setInterval(updatePending, PENDING_ORDERS_PRICE_CHECK_POLL_INTERVAL) | ||
| updatePendingRef.current() | ||
| const interval = setInterval(() => updatePendingRef.current(), PENDING_ORDERS_PRICE_CHECK_POLL_INTERVAL) |
There was a problem hiding this comment.
updatePending function is the culpit of the re-renders that cause too many queries
Co-authored-by: Leandro <[email protected]>
anxolin
force-pushed
the
reduce-queries-from-unfillable-updater
branch
from
2aee35c
to
77d6972
Compare
anxolin
changed the base branch from
fix-verified-quote-issue
to
release/1.47.0
alfetopito
approved these changes
Sep 7, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR reduces drastically the number of quotes we do.
Why?
Because the Unfillable Orders Updater was being "re-renderer" often. The re-rendering was making us to re-start the polling count (reset the timer), but also we were eagerly querying the quote!!
Ok, but why so many re-renders
It turns out, the updater had a
useEffectwho create the polling logic. ThisuseEffectdepended on a function to do the actual update. This function changes often. However, even if this function changes, we don't want to re-fetch again.Soluton, proposed in this PR
Instead of depending on the function itself, we depend on a reference to it.
This way, we don't need to re-render the
useEffectand the polling works as expected.Before it would send hundreds of requests, specially in case we start to get some 404
Now we do way less in this case:
